feat: add support for http_proxy #175
Conversation
There was a problem hiding this comment.
Thank you @shipperizer for the contribution.
In addition to my comments below, can you please also improve the PR description so that reviewers know why this is needed. Please also specify in there that this PR fixes #125
| @@ -224,7 +224,7 @@ def _push_csr_to_workload(self, csr: str) -> None: | |||
| def _execute_lego_cmd(self) -> bool: | |||
| """Execute lego command in workload container.""" | |||
| process = self._container.exec( | |||
| self._cmd, timeout=300, working_dir="/tmp", environment=self._plugin_config | |||
| self._cmd, timeout=300, working_dir="/tmp", environment=self._common_config | self._plugin_config | |||
There was a problem hiding this comment.
This lib is owned by the lego k8s project. Let's make the change over there, bump the lib version, then pull it here.
There was a problem hiding this comment.
| http_proxy: | ||
| description: URL of the HTTP proxy eg http://proxy.internal:6666, it will set the HTTP_PROXY var in the workload environment | ||
| type: string | ||
| default: '' | ||
| https_proxy: | ||
| description: URL of the HTTPS proxy eg http://proxy.internal:6666, it will set the HTTPS_PROXY var in the workload environment | ||
| type: string | ||
| default: '' | ||
| no_proxy: | ||
| description: Domains that need to be excluded from proxying no_proxy="test.com,test.co.uk", it is a comma separate list | ||
| type: string | ||
| default: '' |
There was a problem hiding this comment.
Just like it is done in GitHub Runners charm and other IS charms, let's use the Juju env variables for this.
Reference
There was a problem hiding this comment.
I'm happy to change it
although using those env variables might be counterproductive
ideally you have 2 layers, one where u use those by default and another where u can override that behaviour
There was a problem hiding this comment.
(I placed the same comment on the lego k8s lib, let's discuss it over there)
There was a problem hiding this comment.
I don't have a strong opinion, but I see there was a discussion here. And the point was made to keep consistency across charms.
In addition using the juju model env vars would allow us to handle this in the lib only without the need to change any of the Lego charms.
|
This has been addressed by #178 and canonical/lego-base-k8s-operator#143 |
No description provided.